-
Notifications
You must be signed in to change notification settings - Fork 664
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow molecule to list scenarios present under molecule directory within a collection #3989
Conversation
22fa5db
to
5171ee5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fact this this passed packaging worries me because we added a new dependency which we failed to declare. That job should have failed. We better find a way to make it fail if we do this to avoid adding hidden deps. I am trying to figure-out how to do this properly...
yeah, looks like it got pulled in from ansible-lint (for the tests) I think we should be able to do this without wcmatch though......or at least wait until we get a issue that can';t be resolved without it. |
I told @ajinkyau to use wcmatch, that is not an issue. The issue is that we did not declare it as a direct dependency. Second issue is why it got installed as I doubt we want to install ansible-lint in normal testing environments. I am still looking for a tool that can help us detect missing direct dependencies but i did not find one that works with modern packaging. |
ansible-lint is a dep for test........could we remove that? I'll look into wcamtch, I'm not familiar enough with it to grok what it does above the std lib |
related, navigator had a similar issue a while back, where a dep was satisfied only in the tests, this was added as a check The goal being to run through some basic functionality, enough to hit the import statements across the codebase to flesh out anything missing in the base reqs |
@ssbarnea @cidrblock Since we've removed support for the |
Related: #3987